-
Notifications
You must be signed in to change notification settings - Fork 37
Implemented with ES6 classes, additional functionality. #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
JavaScript/3-classes.js
Outdated
|
||
} | ||
|
||
searchPos(searchPosition) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше itemAtPos(position)
JavaScript/3-classes.js
Outdated
findFirst(name) { | ||
let current = this.first; | ||
|
||
while (current !== null && current.name !== name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно даже current && current.name !== name
JavaScript/3-classes.js
Outdated
let position = 0; | ||
|
||
if (this.length < 0 || removePosition >= this.length) { | ||
throw new Error('Position out of bounds.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не принято ошибки бросать по таким поводам, лучше вернуть ошибку или false из функции
JavaScript/3-classes.js
Outdated
let current = this.first; | ||
let position = 0; | ||
|
||
if (this.length === 0 || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Форматирование этого длинного условия нужно как-то причесать. Я думаю, что если разобраться в сути, то условие будет покороче.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В общем все ок
JavaScript/3-classes.js
Outdated
} | ||
|
||
return current; | ||
} | ||
|
||
remove(removePosition) { | ||
remove(remPos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше просто pos
, не нужно дублировать имя функции в ее аргументах
Implemented next methods: searchPos, remove, findFirst, findAll, find.